Use mock block factory in breaker tests #122965
Conversation
dnhatn
added
v8.19.0
:Analytics/ES|QL
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
|
||
| public static class EsqlTestPluginWithMockBlockFactory extends EsqlPlugin { | ||
| @Override | ||
| protected BlockFactoryProvider blockFactoryProvider( |
There was a problem hiding this comment.
Can't we do this same thing, but injecting the MockBlockFactory directly without the need of the extra "Provider" layer? From what I see, the BlockFactory is already being injected, so just injecting the mock should be enough? Maybe with this same blockFactoryProvider() override.
It's a "Supplier of Factories", which is a bit :hehe:. If it's absolutely needed, no problem with it. But if we can avoid it... At least I'm not seeing what does it give us exactly 👀
There was a problem hiding this comment.
Sadly, we can't inject a MockBlockFactory for a BlockFactory as it requires exact class names. I tried an alternative that avoids injecting BlockFactory by passing the ExchangeService, but it's less natural than this approach.
ivancea
left a comment
ivancea
left a comment
There was a problem hiding this comment.
Looks good. I don't like having this factory provider, but I guess we don't have a better way until the dependency injections allows that
|
Thanks @ivancea. |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
3 participants
We have two leaks reported in breaker tests, but we do not have their traces. This PR integrates MockBlockFactory for breaker tests to help identify these potential leaks.
Relates #122810